Skip to content

[manila-csi-plugin] Restore --nodeid and --nodeaz as optional overrides#3140

Open
gouthampacha wants to merge 1 commit into
kubernetes:masterfrom
gouthampacha:worktree-restore-nodeid-flags
Open

[manila-csi-plugin] Restore --nodeid and --nodeaz as optional overrides#3140
gouthampacha wants to merge 1 commit into
kubernetes:masterfrom
gouthampacha:worktree-restore-nodeid-flags

Conversation

@gouthampacha

Copy link
Copy Markdown
Contributor

What this PR does / why we need it:

Un-deprecates the --nodeid and --nodeaz flags so they can be used as
alternatives to the metadata service for node identity.

#2734 aligned Manila CSI with how Cinder CSI handles node identity by making
the metadata service mandatory. For Cinder this makes sense: the node must be
a Nova VM for volume attach/detach. Manila doesn't have that constraint. Shares
mount via NFS or CEPHFS on the host, and the node doesn't need to be a Nova
instance.

When --nodeid is set, the flag value is used directly. When unset, the driver
falls back to the metadata service (current behavior). Same for --nodeaz.
OpenStack deployments are unaffected.

Which issue this PR fixes(if applicable):

Fixes #3139

Special notes for reviewers:

Tested end-to-end on a DevStack environment with Manila LVM:

  • With --nodeid=test-node-from-flag: CSINode registers with the flag value,
    PVC binds, share mounts, pod reads/writes data
  • Without --nodeid (default): CSINode registers with the metadata-provided
    UUID, PVC binds, share mounts, pod reads/writes data

Release note:

[manila-csi-plugin] The `--nodeid` and `--nodeaz` flags are no longer
deprecated. When set, they override the metadata service for node identity,
allowing Manila CSI to run on nodes that are not OpenStack VMs.

@kubernetes-prow kubernetes-prow Bot added release-note Denotes a PR that will be considered when it comes time to generate release notes. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. labels Jun 23, 2026
@kubernetes-prow
kubernetes-prow Bot requested review from kayrus and zetaab June 23, 2026 06:57
@kubernetes-prow kubernetes-prow Bot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Jun 23, 2026
@gouthampacha

Copy link
Copy Markdown
Contributor Author

@kayrus @yahor-kurachkin could you review this when you get a chance?

ShareName string `name:"shareName" value:"optionalIf:shareID=." precludes:"shareID"`
ShareAccessID string `name:"shareAccessID" value:"optionalIf:shareAccessIDs=." precludes:"shareAccessIDs"` // Keep this for backwards compatibility
ShareAccessIDs string `name:"shareAccessIDs" value:"optionalIf:shareAccessID=." precludes:"shareAccessID"`
ShareAccessID string `name:"shareAccessID" value:"optional"` // Deprecated: use shareAccessIDs

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

could you please make this change in a separate PR?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ugh, this is my bad, it was a part of a different PR: #3132

}

err = d.SetupNodeService(metadata)
err = d.SetupNodeService(nodeID, nodeAZ, md)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this change needs to be aligned with #3141
@gouthampacha can you check the #3141 as well?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've left a comment on #3141 about the /dev mount scope. If #3141 merges first, I'll rebase this PR on top. The conflict is limited to the metadata init block in main.go and should be straightforward to resolve either way.

@yahor-kurachkin yahor-kurachkin left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a minor comment about the testing approach. Otherwise LGTM.

Comment thread pkg/csi/manila/metadata_test.go
@gouthampacha
gouthampacha force-pushed the worktree-restore-nodeid-flags branch from 653de24 to 29c5534 Compare July 25, 2026 06:27
Un-deprecate the --nodeid and --nodeaz flags so that they can be used
as alternatives to the metadata service for node identity. When set,
the flag values are used directly; when unset, the driver falls back
to the metadata service as before.

This allows Manila CSI to work on nodes that are not OpenStack VMs,
such as bare metal clusters or edge deployments, where the metadata
service is unavailable.
@gouthampacha
gouthampacha force-pushed the worktree-restore-nodeid-flags branch from 29c5534 to 7dfe7bf Compare July 25, 2026 06:37
@kayrus

kayrus commented Jul 25, 2026

Copy link
Copy Markdown
Contributor

/test pull-cloud-provider-openstack-check

@kayrus

kayrus commented Jul 25, 2026

Copy link
Copy Markdown
Contributor

/lgtm
/approve

@kubernetes-prow kubernetes-prow Bot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jul 25, 2026
@kubernetes-prow

Copy link
Copy Markdown
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: kayrus

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@kubernetes-prow kubernetes-prow Bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jul 25, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. lgtm "Looks good to me", indicates that a PR is ready to be merged. release-note Denotes a PR that will be considered when it comes time to generate release notes. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

manila-csi-plugin: Restore --nodeid/--nodeaz flags as alternative to metadata service

3 participants